Improvements and bug fixes for sizeof calculator #236
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@generalmimon , I see that you've started to merge some PRs and close issues. Could you look at my PRs? This one is a good candidate to start. List of all my PRs: https://github.com/pulls?page=1&q=is%3Aopen+is%3Apr+author%3AMingun+archived%3Afalse+org%3Akaitai-io |
Mingun
added a commit
to Mingun/kaitai_struct_tests
that referenced
this pull request
Apr 10, 2023
Mingun
force-pushed
the
sizeof-tests
branch
9 times, most recently
from
bba5067 to
b1bc207
Compare
Mingun
force-pushed
the
sizeof-tests
branch
2 times, most recently
from
bb40c22 to
d00f859
Compare
|
@GreyCat, @generalmimon, I see that you have some activity in the project recently. Could you find a time to review my PRs, for example, this? |
Mingun
force-pushed
the
sizeof-tests
branch
3 times, most recently
from
1e3c413 to
4346a1f
Compare
Mingun
added a commit
to Mingun/kaitai_struct_tests
that referenced
this pull request
Mar 8, 2024
Mingun
added a commit
to Mingun/kaitai_struct_tests
that referenced
this pull request
Jul 16, 2024
Mingun
force-pushed
the
sizeof-tests
branch
2 times, most recently
from
d972411 to
c33618e
Compare
Mingun
force-pushed
the
sizeof-tests
branch
4 times, most recently
from
ff2df58 to
ee12aec
Compare
Part of them already was sealed, so there is no reasons to not seal other
Both ArrayTypeInStream and CalcArrayType the only existing successors of the sealed class TypeDetector so we can do that simplification
Fow now only very basic tests is implemented - only size of built-in types is tested
The following tests failed before the change: [info] Ast$Test: [info] - considers `false and ?` constant *** FAILED *** [info] None was not equal to Some(false) (Ast$Test.scala:67) [info] - considers `? and false` constant *** FAILED *** [info] None was not equal to Some(false) (Ast$Test.scala:74) [info] - considers `true or ?` constant *** FAILED *** [info] None was not equal to Some(true) (Ast$Test.scala:81) [info] - considers `? or true` constant *** FAILED *** [info] None was not equal to Some(true) (Ast$Test.scala:88)
- Forbid negative counts of repetitions in `repeat-expr` - Warn about zero counts in `repeat-expr` - Forbid falsy values in `repeat-until` - Warn about only one iteration in `repeat-until`
Mingun
added a commit
to Mingun/kaitai_struct_tests
that referenced
this pull request
Oct 6, 2024
|
@generalmimon, @GreyCat, there already almost 3 years past. Could you look at this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves situation with
_sizeof/sizeof<>:switch-ontypes with the same size in all arms now correctly have the constant sizeifkeys now correctly influence to class / attributesizeof(fixes_sizeofspecial property does not properly handle conditional fields kaitai_struct#927)repeat-exprkeyrepeat-until: <falsy value>because that will lead to an infinity cycleThis PR is based on the #225 because the last commit (f6238c0) from that PR is needed for correct
switch-onsize calculationsAlso this PR fixes the
expr_sizeof_value_dynamic3.ksytest -- corresponding PR to the tests repository: kaitai-io/kaitai_struct_tests#95. Also, this test was mentioned in the kaitai-io/kaitai_struct#721